fix(webhooks)!: comply with AdCP 3.2 challenge schema - #1897

Ladon verdict: Request changes

Request changes — 2 blocking finding(s)

Blocking findings

• src/adcp/webhooks.py:1103 — Breaking public-API signature change under non-breaking fix: prefix. create_webhook_challenge_payload, challenge_webhook_destination (both in adcp.__all__) and WebhookSender.send_webhook_challenge gain required params seller_agent_url/event_types, raising TypeError for existing callers. Per the repo's mandatory semver-signal rule, this needs fix!:/feat!: or a BREAKING CHANGE: footer plus a migration note; release-please would otherwise cut a patch and ship the break without a major.
• src/adcp/webhooks.py:1444 — challenge_webhook_destination now rejects legacy-only (no RFC 9421 sender) callers, another breaking public-surface change shipped under a non-breaking prefix.

Both blockers share one root cause: a breaking public-API change on the adcp.* namespace without the required semver signal. Fix by relabeling the commit (fix!:/feat!: + migration note), not necessarily by reverting the behavior.

Positives verified by the reviewer: credential never ships (asserted at tests/test_webhook_challenge.py:616), the pre-network signs_with_rfc9421 guard fails closed, and legacy-auth ValueErrors map to invalid_configuration. One non-blocking posture note: WebhookSender.from_jwk now accepts request-signing keys for webhook signing — worth confirming the signing-profile change is sanctioned under AdCP 3.2 upstream.

(Row 1 of the decision table: high findings present → request-changes.)

Blocking findings

• src/adcp/webhooks.py:1103 — Breaking public-API signature change under non-breaking fix: prefix (required params added to public exports; needs fix!/BREAKING CHANGE + migration note)
• src/adcp/webhooks.py:1444 — challenge_webhook_destination now rejects legacy-only callers, a breaking change under a non-breaking prefix

Ladon verdict: Approve

Approve — prior blockers resolved, clean delta.

Both prior high findings are now resolved: the breaking public-API changes to create_webhook_challenge_payload, challenge_webhook_destination, and WebhookSender.send_webhook_challenge (all in adcp.__all__) now ship under fix(webhooks)!: with a BREAKING CHANGE: footer and a Migration section naming all three functions, satisfying the mandatory semver signal (release-please cuts a major).

New delta reviewed clean: the always-signed sender path preserves the body-size cap, reserved-header validation, bounded timeout, and IP-pinned transport; the credential is never shipped (fingerprint-only, asserted at tests/test_webhook_challenge.py:616); send_webhook_challenge fails closed on non-RFC-9421 senders before network I/O; payloads validate against the bundled 3.2 webhook-challenge schema; and the loosened from_jwk only accepts WEBHOOK_ACCEPTED_ADCP_USES already honored by the verifier in base.

No blocking or medium findings. high_risk is true only for two (modified) signing files, but with no medium-or-higher findings on them the modifications are presumed safe — no escalation trigger. No author-team gate; gated_paths is false. Falls through to row 9.

Ladon verdict: Approve

Approve.

Clean pass with no critical/high/medium findings. Walking the decision table top-down:

• Rows 1 (critical/high) — no findings.
• Row 2 (gated_paths) — gated_paths is false; also review_decision is APPROVED. Not triggered.
• Rows 3–5 (high_risk escalations) — high_risk is true but all reasons are (modified) with no medium-or-higher findings, so no deletion and no medium concern on a modified sensitive file. Not triggered.
• Row 6 (sticky escalate) — prior decision was approve, not escalate. Not triggered.
• Row 7 (no-auto-approve team) — no team match.
• Row 8 (3+ mediums) — zero medium findings.
• Row 9 — approve.

Substantive checks confirmed by the reviewer: the breaking change to create_webhook_challenge_payload, challenge_webhook_destination, and WebhookSender.send_webhook_challenge (all public adcp.* exports) correctly carries the fix(webhooks)!: / BREAKING CHANGE semver signal plus a migration note, satisfying the mandatory public-surface semver gate. New payload shape validates against the bundled 3.2.7 core/webhook-challenge.json with matching required set and additionalProperties: false. No credential leakage — legacy Bearer/HMAC auth bound by SHA-256 fingerprint only, verified by an explicit no-leak assertion. Fail-closed RFC 9421 sender requirement matches the schema MUST. Timeouts preserved on the send path. jsonschema is a core dependency so the hard validation requirement is safe.

Ladon verdict: Approve

Approve — clean subsequent pass at the same HEAD as the prior approve.

No critical, high, or medium findings. The reviewer re-reviewed all 10 files and confirmed:

• create_webhook_challenge_payload (webhooks.py:1114-1152) emits AdCP 3.2 required fields and validates against the bundled core/webhook-challenge.json before returning; wire shape matches schema (seller_agent_url/delivery_auth/event_types required, delivery_auth allOf gating credential_fingerprint).
• Legacy Bearer/HMAC credentials are sha256-fingerprinted, never emitted (test asserts credentials absent from serialized payload); challenges fail-closed on non-RFC9421 senders — satisfies the no-credentials-in-payload posture.
• Signing-key posture change matches pre-existing WEBHOOK_ACCEPTED_ADCP_USES / adcp#5555 and the verifier — not spec drift.
• Breaking signature changes carry fix!: + BREAKING CHANGE footer + migration note — semver signal correct.

Decision table: no critical/high (row 1 n/a); gated_paths false and review_decision APPROVED (row 2 n/a); high_risk reasons are only (modified) with no medium finding (rows 3-5 n/a); prior decision was approve so row 6 n/a; no no-auto-approve team match (row 7 n/a); zero medium findings (row 8 n/a). Falls through to row 9 — approve. The high_risk flag covers two modified signing files, but with no medium-or-higher findings the modifications are presumed safe.